Add WithAllParentMasters binary write builder option #547
Conversation
| var masters = new HashSet<ModKey>(); | ||
| var remainingMasters = new Queue<IMasterReferenceGetter>(_mod.MasterReferences); | ||
| while (remainingMasters.Count > 0) | ||
| { | ||
| var master = remainingMasters.Dequeue(); | ||
| masters.Add(master.Master); | ||
| loadOrder.AssertListsMod(master.Master); | ||
|
|
||
| var masterMod = loadOrder.ListedOrder.First(mod => mod.ModKey == master.Master).Mod ?? throw new MissingModException(master.Master); | ||
| foreach (var parent in masterMod.MasterReferences) | ||
| { | ||
| remainingMasters.Enqueue(parent); | ||
| masters.Add(parent.Master); | ||
| } | ||
| } | ||
|
|
||
| // Ensure the masters are in the same order as the load order | ||
| var modKeyOrder = loadOrder.ListedOrder.Select(listing => listing.ModKey); | ||
| var sortedMasters = masters.OrderBy(m => modKeyOrder.IndexOf(m)); | ||
|
|
||
| return this with | ||
| { | ||
| _params = _params with | ||
| { | ||
| _param = _params._param with | ||
| { | ||
| ExtraIncludeMasters = _params._param.ExtraIncludeMasters.EmptyIfNull().And(sortedMasters).Distinct().ToArray() | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we refactor this logic to an internal component that takes in the LO and returns the IEnumerable<ModKey>?
Once it's its own component, it'll be easier to test correctness, too, etc.
Main thing is that this builder call shouldn't be the one specifying the LO as an input parameter. The LO is set via other builder calls. Since not all the info is known at the time of THIS call, the logic written here should be run "lazily" at the end once all the bits are set and the Write is actually called
To get the "laziness", a system like the _loadOrderSetter will need to be added like a _mastersContentSetter. When called during the "pull together" phase, it would pull in the LO set right before, and then use that to calculate the masters. I can do this part if you want, as it's pretty fiddly and error prone
There was a problem hiding this comment.
I looked at the _loadOrderSetter before, but the load order it has just has a limited IModFlags interface or so, which didn't allow master references to be looked at. I didn't want to edit the existing builder code so I went this way for now.
There was a problem hiding this comment.
True true. hmm
Well. I think the idea is that load order is mainly used for ordering. Whereas the Data folder is used for looking up info about mods. So the presence of ILoadOrderGetter<IModFlagsGetter> loadOrder is maybe deceiving?
I think someone like yourself using WithAllParentMasters() would:
- not supply anything in the
WithAllParentMasters()call itself - Would include
WithDataFolder(...)to let the system know where to read mods from to find all the parent masters - Would include
WithLoadOrder(...)to let the system know how to order all the parent masters
Maybe as a followup, we could remove the ILoadOrderGetter<IModFlagsGetter> loadOrder, but I would want to deep dive on it to make sure that's a good cut.
No description provided.